Fixing unstable GitConfig parsing#42
Conversation
…red collection of entries, so in case of two remotes (origin, upstream) one time there would be (origin, upstream), another time (upstream, origin). And since remotes are compared as lists, permutations are assumed as non-equal and produce infinite loop of git log updates. Fixing by wrapping Ini with a sorted map (TreeMap).
|
@claude review pr with inline comments |
|
Claude finished @unv-unv's task in 2m 58s —— View job Review: Fixing unstable GitConfig parsing
Verdict: looks good. This is a solid root-cause fix. I left 2 inline comments:
None of these block merging — the only actionable item is the cosmetic blank-line nit. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| return emptyConfig; | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Nit: the fix leaves a double blank line here — the original already had one blank line after the catch block, and this adds another. Worth collapsing back to a single blank line.
| Pair<Collection<Remote>, Collection<Url>> remotesAndUrls = parseRemotes(ini, GitConfig.class.getClassLoader()); |
| SortedMap<String, Profile.Section> ini; | ||
| try { | ||
| ini = GitConfigHelper.loadIniFile(configFile); | ||
| ini = new TreeMap<>(GitConfigHelper.loadIniFile(configFile)); |
There was a problem hiding this comment.
Good root-cause fix — wrapping in a TreeMap makes section iteration order deterministic, so two remotes (e.g. origin/upstream) always parse into the same list order and the equality check no longer flip-flops, which is what was driving the infinite git-log refresh loop.
Two notes for consideration (both non-blocking):
-
TreeMapvsLinkedHashMap—TreeMapsorts sections alphabetically, which is perfectly deterministic and fine here. If you'd prefer to preserve the on-disk.git/configorder,new LinkedHashMap<>(GitConfigHelper.loadIniFile(configFile))is equally deterministic. Either is correct; just flagging the behavioral choice. -
ini4j
Iniis aMultiMap— copying it into a plainMapcollapses any duplicate section keys to a single entry (last wins), whereasIni.get()returns the first. Real.git/configfiles don't repeat a section name, so this is only an edge case, but worth being aware of.
2 participants
Ini.entrySet returns randomly ordered collection of entries, so in case of two remotes (origin, upstream) one time there would be (origin, upstream), another time (upstream, origin). And since remotes are compared as lists, permutations are assumed as non-equal and produce infinite loop of git log updates. Fixing by wrapping Ini with a sorted map (TreeMap).